fix(kiro): support kiro-cli 1.24+ Q Developer format#288
fix(kiro): support kiro-cli 1.24+ Q Developer format#288ratulsarna merged 2 commits intosteipete:mainfrom
Conversation
- Add parsing for new 'Plan: X' format from kiro-cli 1.24+ - Handle 'managed by admin' cases for enterprise plans - Add isUsageOutputComplete checks for new format - Add 3 test cases for Q Developer plan formats Fixes steipete#287
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c229d803b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Require at least one key pattern to match to avoid silent failures | ||
| if !matchedPercent, !matchedCredits { | ||
| if !matchedPercent, !matchedCredits, !matchedNewFormat { | ||
| throw KiroStatusProbeError.parseError( | ||
| "No recognizable usage patterns found. Kiro CLI output format may have changed.") |
There was a problem hiding this comment.
Reject new-format output that lacks usage data
Because matchedNewFormat now bypasses the “no recognizable usage patterns” error, any output that only contains a Plan: line but omits usage/credits (and does not include the “managed by admin” marker) will be treated as valid and fall back to the default creditsTotal = 50. This means a Q Developer Pro (or any paid) plan that emits the new header-only format will be reported as 0/50 free-tier usage, which is inaccurate. Consider keeping the parse error unless you also saw usage data, or explicitly mark the snapshot as unknown instead of using the free-tier default.
Useful? React with 👍 / 👎.
Address review feedback: only bypass parse error for managed plans that don't expose usage data. Non-managed plans with Plan: header but no usage data will now correctly throw parse error instead of falling back to default 50 credits.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7559522d79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Check if this is a managed/enterprise plan with no usage data | ||
| let isManagedPlan = lowered.contains("managed by admin") | ||
| || lowered.contains("managed by organization") | ||
| || lowered.contains("enterprise") |
There was a problem hiding this comment.
Avoid zeroing usage for enterprise plans with metrics
The managed-plan fallback currently treats any output containing enterprise as unmanaged usage-less data, so a new-format response like Plan: Q Developer Enterprise that still includes covered in plan/percent lines will be forced into the early-return snapshot with all usage fields set to 0. That silently discards real parsed credits and reset data for enterprise users, producing incorrect usage in production. Restrict this fallback to explicit managed-by markers (or only when no usage markers were parsed) before returning zeros.
Useful? React with 👍 / 👎.
|
Thanks @kilhyeonjun ! |
Summary
Fixes #287
kiro-cli 1.24.1 changed its output format due to AWS Q Developer rebranding. This PR adds support for the new format.
Changes
Plan: Xformat from kiro-cli 1.24+managed by admincases for enterprise/Pro plansisUsageOutputCompletechecks for new formatOld Format (kiro-cli < 1.24)
New Format (kiro-cli 1.24+)
Testing
Added 3 new test cases:
parsesQDeveloperManagedPlan()- Tests "Plan: Q Developer Pro" with "managed by admin"parsesQDeveloperFreePlan()- Tests "Plan: Q Developer Free"parsesNewFormatWithANSICodes()- Tests ANSI escape code handling